Skip to content

Conversation

@faculerena
Copy link

Fixed as_substr function to check for matching prefixes too. This fixes this issue where td::s is matched instead of std::. This same behaviour can be seen with time and tokio::time and similar cases where the error import starts with the same prefix as the suggestion.

use sync;
fn main() {}
error[E0432]: unresolved import `sync`
 --> /tmp/test_import.rs:1:5
  |
1 | use sync;
  |     ^^^^ no `sync` in the root
  |
help: consider importing this module instead
  |
1 | use std::sync;
  |      +++++

After the changes:

error[E0432]: unresolved import `sync`
 --> /tmp/test_import.rs:1:5
  |
1 | use sync;
  |     ^^^^ no `sync` in the root
  |
help: consider importing this module instead
  |
1 | use std::sync;
  |     +++++

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 24, 2025
@faculerena faculerena force-pushed the master branch 3 times, most recently from 6e1a961 to 3a0a76b Compare October 24, 2025 05:47
@rust-log-analyzer

This comment has been minimized.

@Kivooeo
Copy link
Member

Kivooeo commented Oct 24, 2025

Well, I guess we can't modify this as_substr, seems like even smallest change breaks everything else,

I guess we should somehow modify this function

fn throw_unresolved_import_error(

This function was pointed by -Ztrack-diagnostics

error[E0432]: unresolved import `sync`
 --> /home/gh-Kivooeo/test_/src/main.rs:2:5
  |
2 | use sync;
  |     ^^^^ no `sync` in the root
  |
  = note: -Ztrack-diagnostics: created at compiler/rustc_resolve/src/imports.rs:768:24
help: consider importing this module instead
  |
2 | use std::sync;
  |      +++++

This one generates a final suggestion, but I don't really sure how this tide with as_substr (haven't checked stack trace yet, but I guess you did this already, otherwise I have no idea how you found this as_substr function)

So, what is my idea about, in the place where we generating suggestion (I guess this is function above, but I'm not 100% sure) we need to check what we got:

  1. crate::
  2. rate::c or ate::cr

so if we got first we are fine with that, if we got second then we should somehow move cr to the start, so we will get crate::, here we have some advantage because we always know that everything after :: was stripped from start, so it should be moved

I will be honest that I have no idea how to implement that in that function, I maybe will took another look but later, so there is just some of my thoughts on the core idea how this should work

@Kivooeo
Copy link
Member

Kivooeo commented Oct 24, 2025

diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 17cd466f96b..2ed4e281adb 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -306,6 +306,18 @@ fn replaces_meaningful_content(&self, sm: &SourceMap) -> bool {
 /// `BB` is. Return the length of the prefix, the "trimmed" suggestion, and the length
 /// of the suffix.
 fn as_substr<'a>(original: &'a str, suggestion: &'a str) -> Option<(usize, &'a str, usize)> {
+    if suggestion.contains("::")
+        && ["core", "std"].iter().any(|&prefix| suggestion.starts_with(prefix))
+        && suggestion.ends_with(original)
+        && let Some(c1) = suggestion.chars().nth(0)
+        && let Some(c2) = original.chars().nth(0)
+        && c1 == c2
+    {
+        let prefix_len = suggestion.len() - original.len();
+        let prefix = &suggestion[..prefix_len];
+        return Some((0, prefix, original.len()));
+    }
+
     let common_prefix = original
         .chars()
         .zip(suggestion.chars())

Btw, just decided to check will this work or not... and sadly it is, all tests passed, and it gave correct suggestion

But... I'd consider this as a last resort if we can't come up with a better solution

@faculerena
Copy link
Author

Yeah, I was just trying to generalize that solution, as that does not catch third party crates. But yeah, it looks like the solution is that way.

@Kivooeo
Copy link
Member

Kivooeo commented Oct 24, 2025

I guess it's possible to remove core and std check from here, I was just lazy to rerun tests once again to test it

It's a bit late for me atm (3 am) so I would able to test it without this check later today

Or if you want to test it yourself, you can copy this into your rustc fork and run ./x test ui you will see if there any regressions in test suite

But I'd be happier to see an implementation of my idea above, I also will try to investigate this direction more later on today

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
To only update this specific test, also pass `--test-args test-attrs/inaccessible-test-modules.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/test-attrs/inaccessible-test-modules.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/test-attrs/inaccessible-test-modules" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "--test"
stdout: none
--- stderr -------------------------------
error[E0432]: unresolved import `main`
##[error]  --> /checkout/tests/ui/test-attrs/inaccessible-test-modules.rs:5:5
   |
LL | use main as x; //~ ERROR unresolved import `main`
   |     ^^^^^^^^^ no `main` in the root

error[E0432]: unresolved import `test`
##[error]  --> /checkout/tests/ui/test-attrs/inaccessible-test-modules.rs:6:5
   |
LL | use test as y; //~ ERROR unresolved import `test`
   |     ^^^^^^^^^ no `test` in the root
   |
help: consider importing this module instead
   |
LL | use test::test as y; //~ ERROR unresolved import `test`
   |     ++++++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0432`.

@faculerena
Copy link
Author

faculerena commented Oct 24, 2025

This last potential fix I pushed has only one test failing. I missed that for some reason locally, I think I'm missing something for proper testing.

13	help: consider importing this module instead
14	   |
15	LL | use test::test as y;
-	   |         ++++++
+	   |     ++++++
17	
18	error: aborting due to 2 previous errors
19	

I think this is an acceptable compromise (as is the case when there's a nested foo::foo), but I'm no one to decide over this. We could add a check for cases when the suggestion is foo:: and the original is foo

diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 9cd34a8747f..5e4bd514af8 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -312,7 +312,10 @@ fn as_substr<'a>(original: &'a str, suggestion: &'a str) -> Option<(usize, &'a s
         && suggestion.len() > original.len()
     {
         let prefix = &suggestion[..suggestion.len() - original.len()];
-        if prefix.ends_with("::") && suggestion.chars().next() == original.chars().next() {
+        if prefix.ends_with("::")
+            && suggestion.chars().next() == original.chars().next()
+            && suggestion.trim_suffix("::") != original
+        {
             return Some((0, prefix, original.len()));
         }
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants